Skip to content

fix(templates): Fixing overlapping of callouts for v8 and v9 components#8897

Merged
preetriti1 merged 7 commits intomainfrom
priti/templates
Mar 12, 2026
Merged

fix(templates): Fixing overlapping of callouts for v8 and v9 components#8897
preetriti1 merged 7 commits intomainfrom
priti/templates

Conversation

@preetriti1
Copy link
Contributor

@preetriti1 preetriti1 commented Mar 6, 2026

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

"What: Panels (QuickView/CreateWorkflow and template overview/single template views) now mount to a provided container (mountNode) so v8 callouts render correctly inside v9 panels.
Why: to prevent callouts/overlays from appearing underneath new panel layer structure and fix overlay/positioning issues.
TODO/Follow-up: The CreateWorkflow panel changing CSS on open (making it non-auto-dismissible) is tracked in ISSUE-#8903 and will be addressed separately (P2)."

Impact of Change

  • Users: connection creation experience fixed in templates
  • Developers: Update panel usage to mount to a node instead of free flowing to make sure v8 components work fine inside callouts
  • System: Not much

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in:

Contributors

@Eric-B-Wu @rllyy97

Screenshots/Videos

Copilot AI review requested due to automatic review settings March 6, 2026 23:58
@github-actions
Copy link

github-actions bot commented Mar 6, 2026

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(templates): Fixing overlapping of callouts for v8 and v9 components
  • Issue: None blocking. The title is descriptive and follows conventional commit style.
  • Recommendation: (Optional) You can make it slightly more specific about the main technical change for easier triage, e.g. fix(templates): mount template panels to container to prevent v8/v9 callout overlap.

Commit Type

  • Properly selected (fix).
  • Note: Exactly one commit type is selected in the PR body which is correct.

Risk Level

  • The PR body selects Medium, and the repo labels include risk:medium.
  • Assessment: The code diff touches multiple UI panel components, adds mountNode plumbing and style changes (position: fixed), and introduces many tests. Those UI/layout changes justify a Medium risk classification — the assigned risk matches my assessment.

What & Why

  • Current: "What: Panels (QuickView/CreateWorkflow and template overview/single template views) now mount to a provided container (mountNode) so v8 callouts render correctly inside v9 panels.\nWhy: to prevent callouts/overlays from appearing underneath new panel layer structure and fix overlay/positioning issues.\nTODO/Follow-up: The CreateWorkflow panel changing CSS on open (making it non-auto-dismissible) is tracked in ISSUE-Overlap panel changes to non-modal type panel and cannot be dismissed when moved to create connection #8903 and will be addressed separately (P2)."
  • Issue: Clear and concise. The TODO/follow-up is useful.
  • Recommendation: (Optional) Add a short note listing the primary files changed (e.g., CreateWorkflowPanel, QuickViewPanel, TemplateOverview/TemplateView) so reviewers know where runtime effects are most likely.

Impact of Change

  • Impact section exists and lists Users/Developers/System.
  • Issue: The "System" line currently says "Not much" — that is acceptable but brief.
  • Recommendation:
    • Users: Connection creation UX and template callout overlays fixed — keep as-is.
    • Developers: Note the new pattern: panels should accept mountNode and consumers must provide a container ref. Mention whether mountNode is now required (I see mountNode is typed as HTMLElement | null but not optional in some components) so callers must pass it — clarify in body if this is an API/prop contract change.
    • System: You can expand to note potential z-index/positioning implications (panels now position: fixed) and any accessibility or layering considerations.

Test Plan

  • Assessment: Unit tests, E2E, and manual testing are all checked in the PR body.
  • Verification: The code diff includes updated e2e (e2e/templates/createWorkflowPanel.spec.ts) and many unit test files added/updated under libs/.../tests which matches the claim.
  • Recommendation: Add a short note about the environment used for manual testing (browser/OS) or CI job names if relevant.

Contributors

  • Contributors listed: @Eric-B-Wu @rllyy97 — good.
  • Recommendation: If there were more contributors (design/PM), consider acknowledging them as well.

⚠️ Screenshots/Videos

  • Assessment: Not provided. For a UI/layout change that affects overlays/callouts, consider adding a before/after screenshot or short gif to help reviewers quickly validate the visual fix.
  • Recommendation: Add at least one before/after screenshot showing callout overlap vs. resolved state.

Summary Table

Section Status Recommendation
Title Optional: make title slightly more specific (mention mountNode / panels).
Commit Type No change needed.
Risk Level Medium is appropriate given UI/layout changes.
What & Why Optional: list primary files changed for quick reference.
Impact of Change Clarify that mountNode is now expected at callers; expand "System" detail.
Test Plan Tests are present in diff; mention manual test environment.
Contributors Good.
Screenshots/Videos ⚠️ Add before/after visuals for UI changes.

Final Message
This PR passes the PR title and body template checks. The risk level declared (Medium) matches the changes introduced by the diff. A few optional improvements will speed review and reduce follow-up questions:

  • Clarify in Impact/Developers that mountNode is now part of the panel API (callers must pass a container ref) and whether this is backward-compatible. If mountNode is required, call this out explicitly.
  • Add one or two before/after screenshots or a short video showing the overlay/callout behavior before and after the change.
  • Optionally make the title slightly more specific to mention mounting panels to a container (helps reviewers and release notes).
  • Add brief manual test environment details (browser/OS/CI pipeline used).

Please update the PR description with any clarifications above if desired, then proceed. Thank you for the thorough tests and clear explanation — this looks ready for functional review/merge from a PR-description perspective.


Last updated: Thu, 12 Mar 2026 19:30:32 GMT

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an overlay/stacking issue in the Templates UI where Fluent UI v8 components (dropdowns, callouts, comboboxes) were not properly displaying inside Fluent UI v9 panels. The fix mounts the v9 Drawer components into a specific container DOM node instead of the document body, using the mountNode prop, and adds position: 'fixed' to the drawer styles to ensure correct overlay behavior.

Changes:

  • Added mountNode prop to QuickViewPanel and CreateWorkflowPanel, wrapping each usage in a position: relative container whose DOM node is passed as the mount target
  • Changed outer wrapper from fragment (<>) to <div> in several view components to support proper DOM containment
  • Added position: 'fixed' to both drawer style definitions to fix overlay positioning

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
quickViewPanel.tsx Added required mountNode: HTMLElement | null prop; passes it to the Fluent UI v9 Drawer; sets type={'overlay'}
quickViewPanel.styles.ts Added position: 'fixed' to the drawer style
createWorkflowPanel.tsx Added required mountNode: HTMLElement | null prop; passes it to the Fluent UI v9 Drawer; sets type={'overlay'}
createWorkflowPanel.styles.ts Added position: 'fixed' to the drawer style
templateview.tsx Wraps panels in a position: relative div with a useRef to supply mountNode; imports useRef
templateoverview.tsx Same pattern as templateview.tsx; uses useRef for container div passed as mountNode
templatesfullgalleryview.tsx Wraps QuickViewPanel/CreateWorkflowPanel in ref-tracked div; adds unused panelRef type to WorkflowView

@github-actions
Copy link

github-actions bot commented Mar 7, 2026

📊 Coverage Check

The following changed files need attention:

⚠️ libs/designer/src/lib/ui/panel/templatePanel/createWorkflowPanel/createWorkflowPanel.tsx - 78% covered (needs improvement)
⚠️ libs/designer/src/lib/ui/templates/templateoverview.tsx - 3% covered (needs improvement)
⚠️ libs/designer/src/lib/ui/templates/templateview.tsx - 9% covered (needs improvement)

Please add tests for the uncovered files before merging.

@preetriti1 preetriti1 enabled auto-merge (squash) March 12, 2026 19:29
@preetriti1 preetriti1 merged commit 3e2af9c into main Mar 12, 2026
14 of 17 checks passed
@preetriti1 preetriti1 deleted the priti/templates branch March 12, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:medium Medium risk change with potential impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants